Draft
Conversation
- Remove unused #include "logging.hpp" from status.hpp (50+ files no longer transitively pull in spdlog). Add minimal #include <fmt/format.h> for fmt::formatter<StatusCode> specialization, with @fmtlib dep in BUILD. - Remove #include <fmt/ranges.h> from logging.hpp; add it only to the 4 files that actually format containers (imagegen_init.cpp, openai_completions.cpp, assisted_decoding_test.cpp, llmnode_test.cpp). - Move <spdlog/sinks/basic_file_sink.h> and <spdlog/sinks/stdout_sinks.h> from logging.hpp to logging.cpp (sinks only needed in configure_logger()). - Add explicit #include "logging.hpp" to color_format_configuration.cpp and precision_configuration.cpp that relied on the transitive include. Verified: //src:ovms and //src:ovms_test build with mp_on_py_on, mp_on_py_off, and mp_off_py_off configs.
Removed #include logging.hpp from 12 headers that had zero logging symbol usage. Added explicit #include <fmt/format.h> to 3 headers that have fmt::formatter specializations (grpc_utils.hpp, modelversionstatus.hpp, nodeinfo.hpp). Added explicit #include logging.hpp to 2 files that lost transitive include (tensor_conversion.hpp, tensor_conversion_common.cpp). Updated BUILD deps: replaced libovmslogging with @fmtlib where only fmt was needed.
…odel.hpp - Removed #include <openvino/openvino.hpp>, replaced with forward declaration of ov::Core (only used as unique_ptr member) - Removed #include <spdlog/spdlog.h> (unused in header) - Removed #include <sys/stat.h> (unused in header) - Replaced #include model.hpp with modelconfig.hpp + forward decl of Model. Moved modelFactory() body to modelmanager.cpp. - Added #include <openvino/openvino.hpp> to modelmanager.cpp - Added #include model.hpp to 4 .cpp files and 4 test files that used Model via shared_ptr through the transitive include. This eliminates the entire OpenVINO umbrella header and spdlog from the 17 translation units that include modelmanager.hpp.
…name - Removed #include <fstream> (unused in header, added to .cpp) - Removed #include <set> and dead static member configAllowedLayouts (declared but never defined or used anywhere) - Removed #include anonymous_input_name.hpp, moved isShapeAuto(), isShapeAnonymous(), isShapeAnonymousFixed(), anyShapeSetToAuto() bodies from inline in header to modelconfig.cpp - Added #include <fstream> and anonymous_input_name.hpp to .cpp
…to decouple mediapipe from ModelManager - Create ServableNameChecker interface (servableExists) and MetricProvider interface (getMetricRegistry, getMetricConfig) - ModelManager inherits both interfaces - MediapipeFactory uses unique_ptr, forward-declared in modelmanager.hpp - mediapipegraphdefinition.hpp/cpp depend on ServableNameChecker, not ModelManager - mediapipefactory.hpp/cpp depend on interfaces, not ModelManager - Remove dead parameters: retire(), create(), retireOtherThan(), revalidatePipelines() - Add explicit mediapipefactory.hpp includes where needed (4 src + 3 test files)
…hecking - Add ServableType enum (Model, Pipeline, Mediapipe) with bitmask operators - servableExists() accepts ServableType check param to specify which types to check - Mediapipe validate: checks Model | Pipeline (skips self) - DAG validate: checks Model | Mediapipe (skips self), removes mediapipefactory.hpp include - Model loading: checks Pipeline | Mediapipe (skips self), removes #if MEDIAPIPE_DISABLE guards - All three servable types now use the same unified servableExists() function
…apper - Change PipelineFactory member from value to unique_ptr in ModelManager - Forward-declare PipelineFactory, Pipeline, ModelInstance, ModelInstanceUnloadGuard in modelmanager.hpp - Move pipeline_factory.hpp include from modelmanager.hpp to modelmanager.cpp - Remove template createPipeline wrapper from ModelManager - Callers now use getPipelineFactory().create() directly - Add explicit pipeline_factory.hpp includes to files that need it
…ableType::Pipeline
- Add ServableDefinition pure virtual interface (getName, isAvailable) - Add SingleVersionServableDefinition bridging ServableDefinition + Servable - Model implements ServableDefinition directly - PipelineDefinition inherits SingleVersionServableDefinition (removes pipelineName member) - MediapipeGraphDefinition inherits SingleVersionServableDefinition (removes name member) - Add Bazel targets for new headers
…cades - Add ModelManager::findServableDefinition() centralizing model->pipeline->mediapipe lookup - Refactor KFS getModelReady to use findServableDefinition (removes nested #if blocks) - Refactor KFS ModelMetadataImpl to use findServableDefinition - Refactor model_service getModelStatus to use findServableDefinition - Refactor CAPI OVMS_GetServableState to use findServableDefinition - Remove unused duplicate BUILD targets: libovmshttpservermodule, libovms_dags_pipelinedefinition - Fix copyright year in single_version_servable_definition.hpp
- Expand SingleVersionServableDefinition with virtual methods: getStatus(), getInputsInfo(), getOutputsInfo(), getMetricReporter(), notLoadedYetCode(), notLoadedAnymoreCode() - Unified waitForLoaded() in SingleVersionServableDefinition base class - Add StatusMetricReporter abstract base to model_metric_reporter.hpp - Unify ServableDefinitionUnloadGuard (replaces PipelineDefinitionUnloadGuard + MediapipeGraphDefinitionUnloadGuard) - Remove mediapipe includes from model_service.cpp and capi.cpp - Unify KFS buildResponse for pipeline/mediapipe paths - Add getServableDefinitionNames() to ModelManager - Add tensorinfo_fwd.hpp for forward declarations - Fix dangling reference: use getName() instead of constructor parameter in PipelineDefinition and MediapipeGraphDefinition constructors
…forward-declare servable types in mediapipegraphdefinition.hpp, remove iostream
…w OpenVINO include in kfs_grpc_inference_service.hpp, move mediapipe parse_text_proto/status to cpp
…e KFS deps from core Phase 3E: Split libovms_mediapipe from ovms_lib into separate target Phase 3F: Extract NodeInitializer registry pattern from initializeNodes() - Created node_initializer.hpp/.cpp with NodeInitializer interface + registry - Created graph_side_packets.hpp extracted from mediapipegraphdefinition.hpp - 7 self-registering concrete initializers in their calculator directories - Refactored ~250 line God function to ~15 line registry loop Phase 3G: Split frontend executors from core mediapipe - libovms_mediapipe (core: factory, definition, executor, config) - libovms_mediapipe_kfs_executor (kfs_graph_executor_impl) - libovms_mediapipe_http_executor (http_graph_executor_impl) Phase 3H: Remove dead KFS/TFS deps from core mediapipe - Removed 5 dead includes (zero symbols used) - Replaced //:ovms_dependencies with specific rapidjson + mediapipe deps - Calculator deps stay at ovms_lib level (not in libovms_mediapipe) - Node initializers use logging.hpp instead of direct spdlog - Fixed missing rapidjson/document.h in rerank_servable.hpp
…+includes in new files
… in rerank and embeddings servable headers
…to namespace ovms
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Build Optimization: Reduce Dependencies, Compilation Unit Sizes, and Improve Build Graph Structure
Goal
Minimize transitive dependencies and preprocessed translation unit (TU) sizes across the OVMS codebase. Heavy headers like
modelmanager.hpp,logging.hpp,status.hpp, and protocol-specific includes were being pulled into dozens of translation units that didn't need them. This inflated build times, reduced build parallelism, and made the dependency graph unnecessarily coupled.Key results (measured on
//src:ovmstarget, mp_on_py_on, usingtools/compare_build_metrics.py --scopes all,non-test,test):Primary KPI view(non-test files only):
Scope comparison (dependencies):
Dependency buckets (non-test):
PP-line buckets (non-test TU view):
Top individual TU reductions:
Approach — High Level
1. Header Slimming via Forward Declarations and Include Discipline
Targeted the most included headers and removed unnecessary transitive dependencies:
status.hpp: Removed#include "logging.hpp"(50+ TUs no longer transitively pull in spdlog). Added minimal<fmt/format.h>for thefmt::formatter<StatusCode>specialization.logging.hpp: Moved spdlog sink headers tologging.cpp(only needed inconfigure_logger()). Removed unused<fmt/ranges.h>(added only to 4 files that actually format containers). Removedlogging.hppfrom 12 headers that had zero logging symbol usage.modelmanager.hpp: Removed<openvino/openvino.hpp>(replaced with forward-decl ofov::Core), removed<spdlog/spdlog.h>,<sys/stat.h>,model.hpp. MovedmodelFactory()body to.cpp. This eliminates the entire OpenVINO umbrella header from 17+ TUs.modelconfig.hpp: Removed<fstream>,<set>,anonymous_input_name.hpp. MovedisShapeAuto()/isShapeAnonymous()inline bodies to.cpp.pipelinedefinition.hpp: Removed protobuf includes, narrowed OpenVINO include scope.kfs_grpc_inference_service.hpp: Narrowed OpenVINO include.mediapipegraphdefinition.hpp: Forward-declared servable types, removediostream, movedparse_text_protoandstatusincludes to.cpp.2. Interface Extraction — Decoupling Subsystems from ModelManager
Introduced lightweight abstract interfaces so that subsystems depend on narrow contracts instead of the full
ModelManager:ServableNameCheckerinterface (servableExists()): Used by mediapipe, DAGs, and model loading code for name collision checking. Replaces directModelManagerdependency inmediapipegraphdefinition.hpp/.cppandmediapipefactory.hpp/.cpp.MetricProviderinterface (getMetricRegistry(),getMetricConfig()): Decouples metric access from ModelManager.ServableTypeenum with bitmask operators:servableExists()now accepts aServableTypeparameter (Model, Pipeline, Mediapipe) to specify which types to check, eliminating#if MEDIAPIPE_DISABLEguards and direct factory access.ServableDefinitionhierarchy:ServableDefinition— pure virtual interface (getName,isAvailable)SingleVersionServableDefinition— bridgesServableDefinition+Servable, provides unifiedwaitForLoaded(),getStatus(),getInputsInfo(),getOutputsInfo(),getMetricReporter()etc.Model,PipelineDefinition,MediapipeGraphDefinitionall fit into this hierarchyfindServableDefinition()in ModelManager centralizes the model→pipeline→mediapipe lookup, replacing duplicated resolution cascades across 5+ frontend files.ServableDefinitionUnloadGuardreplaces separatePipelineDefinitionUnloadGuard+MediapipeGraphDefinitionUnloadGuard.StatusMetricReporterabstract base: Decouples metric reporting from model-specific reporter.3. Mediapipe Target Decomposition
Split the mediapipe code from the monolithic
ovms_libtarget into focused Bazel targets://src/mediapipe_internal:libovms_mediapipe— Core mediapipe: factory, definition, executor, config. Dependencies restricted to specific//src:targets (no catch-allovms_dependencies).//src/mediapipe_internal:mediapipe_utils— String parsing utilities for mediapipe config.//src/mediapipe_internal:node_initializer— Registry + interface for calculator initialization.libovms_mediapipe_kfs_executorandlibovms_mediapipe_http_executoras separate targets (KFS/HTTP graph executor impls). These depend on protocol-specific code; the corelibovms_mediapipedoes not.ovms_liblevel (they are the integration point that links everything together).4. NodeInitializer Registry Pattern
Replaced a ~250-line
initializeNodes()function inmediapipegraphdefinition.cpp(a God function with#ifdefblocks and direct coupling to every subsystem) with a self-registering plugin pattern:NodeInitializerinterface:initialize(GraphSidePackets&),deinitialize(GraphSidePackets&),name().NodeInitializerRegistrysingleton: subsystems register themselves at static init time.llm_node_initializer.cpp,embeddings_node_initializer.cpp,rerank_node_initializer.cpp,python_node_initializer.cpp,image_gen_node_initializer.cpp,stt_node_initializer.cpp,tts_node_initializer.cpp.src/llm/,src/embeddings/) and self-registers via a global constructor. The core mediapipe code has zero knowledge of specific subsystems.initializeNodes()body becomes a ~15-line loop:for (auto& init : registry) init->initialize(packets);.5. Dead Include / Dead Code Cleanup
mediapipegraphdefinition.cpp).PipelineFactory::pipelineDefinitionExists()(replaced by unifiedservableExists()).configAllowedLayoutsstatic set frommodelconfig.hpp.createPipelinewrapper from ModelManager.PipelineFactoryandMediapipeFactorytounique_ptrmembers in ModelManager (forward-declared in header).Commit History
2dd4e89status.hppfrom spdlog/fmt; slim downlogging.hpp8c5590clogging.hppfrom 12 headers with zero logging symbolsdd6571emodelmanager.hpp— remove OpenVINO, spdlog, sys/stat, model.hpp7bc73c2modelconfig.hpp— remove fstream, set, inline function bodies08cd7e0ServableNameCheckerandMetricProviderinterfaces948229fServableTypeflags for unifiedservableExists()1aa6003PipelineFactorytounique_ptr, forward-declare in headerb7d3f4epipelineDefinitionExists, replaced byservableExistsa09dd9fServableDefinitionhierarchy (Phase 1)a9699eefindServableDefinition, refactor frontend resolution cascadesc9e6bfbSingleVersionServableDefinition, unify guards + waitForLoaded10bc0ef1e888cf7ff32a5f0daff0580ebdsrc/mediapipe_internal/BUILD, fix copyright + includesFiles Changed
~100 files across 16 commits. The changes are strictly additive in terms of functionality (zero behavioral changes) and subtractive in terms of coupling.
New Files
src/servable_name_checker.hppServableNameCheckerinterfacesrc/metric_provider.hppMetricProviderinterfacesrc/servable_type.hppServableTypeenum with bitmask operatorssrc/servable_definition.hppServableDefinitionpure virtual basesrc/single_version_servable_definition.hpp/.cppsrc/servable_definition_unload_guard.hpp/.cppsrc/tensorinfo_fwd.hppsrc/mediapipe_internal/BUILDsrc/mediapipe_internal/node_initializer.hpp/.cppsrc/mediapipe_internal/graph_side_packets.hppsrc/llm/llm_node_initializer.cppsrc/embeddings/embeddings_node_initializer.cppsrc/rerank/rerank_node_initializer.cppsrc/python/python_node_initializer.cppsrc/image_gen/image_gen_node_initializer.cppsrc/audio/speech_to_text/stt_node_initializer.cppsrc/audio/text_to_speech/tts_node_initializer.cpp